Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for compiling with MSVC #97

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mrexodia
Copy link

@mrexodia mrexodia commented May 13, 2022

Mostly a draft to see if there is interest. The changes are quite minimal, the only functional change I made was the mulhshr function because MSVC doesn't support uint128_t. I made a pretty extensive test harness for the MSVC implementation though: https://godbolt.org/z/h9q7hj5P1

If this is something you would merge I can do CI with GitHub Actions for Windows as well.

mrexodia added 2 commits May 13, 2022 14:20
Reference: https://developercommunity.visualstudio.com/t/error-c2872-byte-ambiguous-symbol/93889

The reason is that the Ghidra developers for some reason decided to put "using namespace std;" in their header files.
@CLAassistant
Copy link

CLAassistant commented May 13, 2022

CLA assistant check
All committers have signed the CLA.

@Boyan-MILANOV
Copy link
Collaborator

Boyan-MILANOV commented May 13, 2022

Thank you so much for this PR and for your interest in Maat!

Everything looks good to me. It would be really amazing if you could integrate windows in the Github Actions CI before we merge this. Also, is there anything specific worth mentioning in BUILDING.md or HACKING.md for Windows builds?

@mrexodia
Copy link
Author

Not really, the only thing I did was use vcpkg to get GMP. I can add some instructions, but there’s nothing platform specific.

I’ll try also building with lief and z3 for completeness sake and then add GH actions.

@Boyan-MILANOV
Copy link
Collaborator

Awesome! No need to update the documentation then. Looking forward to seeing progress on this. Don't hesitate to ask if you have more questions.

@mrexodia
Copy link
Author

I ran into some issues compiling maat with z3 and lief, will get back to it once I figure out what's going on.

@ekilmer
Copy link
Collaborator

ekilmer commented May 18, 2022

@mrexodia Not sure if you're planning to use vcpkg for Z3 and LIEF, but I have pending PR for LIEF on vcpkg here[1]. It's pending/blocked because LIEF requires mbedtls v3 but other vcpkg ports require mbedtls v2 and aren't the easiest to upgrade/support both major versions 😞 . The failing tests are related to the other vcpkg ports that don't support mbedtls v3.

Moreover, I think the move to use vcpkg for all of CI would make building dependencies easier when supporting multiple OSes. I plan to work on this when I find the time, but I also need to figure out how to ensure OS compatibility with the native Python wheels, i.e. adding -mmacosx-version-min=10.15 to vcpkg's triplet file (most likely) and cross-compiling for Mac M1.

[1] microsoft/vcpkg#22957

@mrexodia
Copy link
Author

Thanks! For vcpkg it should be possible to use manifest mode and then an overlay for mbedtls v3 and lief. I was trying to stay compatible with the current LIEF version used, but this version doesn't compile with MSVC so I will upgrade to the latest LIEF/Z3 and provide vcpkg overlays.

@ekilmer
Copy link
Collaborator

ekilmer commented Jul 7, 2022

@mrexodia I finally found some time to work on vcpkg support for sleigh and have opened a draft PR for maat to use dependencies all from vcpkg here #128.

I also tested the vcpkg integration on Windows and the dependencies all seem to build fine. I also merged this branch into a test branch with the vcpkg integration, but I'm getting some additional errors in Maat that I think have been introduced since you first opened this PR.

  1. There are some complaints about strict strings from the generated https://github.com/trailofbits/maat/blob/master/generate/src/include/maat/config.hpp.in file, which can be fixed with the following patch
diff --git a/generate/src/include/maat/config.hpp.in b/generate/src/include/maat/config.hpp.in
index 3de9b6e..6d0266c 100644
--- a/generate/src/include/maat/config.hpp.in
+++ b/generate/src/include/maat/config.hpp.in
@@ -1,8 +1,8 @@
 #ifndef MAAT_CONFIG_H
 #define MAAT_CONFIG_H

-static constexpr char* maat_install_prefix = "@CMAKE_INSTALL_PREFIX@";
-static constexpr char* maat_specfile_dir_prefix = "@maat_INSTALL_DATADIR@/@spec_out_prefix@";
+static constexpr auto maat_install_prefix = "@CMAKE_INSTALL_PREFIX@";
+static constexpr auto maat_specfile_dir_prefix = "@maat_INSTALL_DATADIR@/@spec_out_prefix@";

 #include <filesystem>
 #include <list>
  1. There are more strict string errors in the Python code (I set -Dmaat_BUILD_PYTHON_BINDINGS=OFF to silence that for now...)
  2. There are some LIEF errors that I haven't yet looked into but I assume are due to upgraded LIEF
C:\src\maat\src\loader\loader_lief_elf.cpp(70,13): error C3861: 'S_ISREG': identifier not found [C:\src\maat\build\dev-win64\maat_maat.vcxproj]
C:\src\maat\src\loader\loader_lief_elf.cpp(76,18): error C3861: 'S_ISDIR': identifier not found [C:\src\maat\build\dev-win64\maat_maat.vcxproj]

Feel free to mess around with the vcpkg integration. I've left some TODOs in #128 of what I was planning, but I won't get to working on that again for 2 weeks or so.

@ekilmer ekilmer mentioned this pull request Jul 7, 2022
6 tasks
@mrexodia
Copy link
Author

mrexodia commented Jul 7, 2022

Thanks for picking this up, I lost track of it. I have the PR marked as editable by collaborators so feel free to push any fixes you might have. Otherwise I’ll pick it up some time in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants